Skip to content

Fix GH-23088: Stack overflow when comparing deeply nested arrays - #23090

Merged
arnaud-lb merged 2 commits into
php:PHP-8.4from
lazerg:fix/gh-23088-array-compare-stack-limit
Aug 10, 2026
Merged

Fix GH-23088: Stack overflow when comparing deeply nested arrays#23090
arnaud-lb merged 2 commits into
php:PHP-8.4from
lazerg:fix/gh-23088-array-compare-stack-limit

Conversation

@lazerg

@lazerg lazerg commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Comparing two deeply nested arrays recurses through zend_compare_arrays -> zend_compare_symbol_tables -> zend_hash_compare once per nesting level, and nothing bounds that recursion. zend_hash_compare() only guards against cycles, so a non-cyclic array a few tens of thousands of levels deep runs the C stack out and the process dies with a segfault. === crashes the same way through zend_is_identical().

Both now check the stack limit before descending and throw an Error instead, the same way zend_std_compare_objects() already handles the object case.

Fixes GH-23088

@lazerg
lazerg force-pushed the fix/gh-23088-array-compare-stack-limit branch 3 times, most recently from aa11ff5 to 983de1f Compare August 6, 2026 16:01
@lazerg
lazerg force-pushed the fix/gh-23088-array-compare-stack-limit branch from 983de1f to e0c17b3 Compare August 6, 2026 16:19
@Girgias
Girgias requested a review from arnaud-lb August 7, 2026 10:35
Comment thread Zend/zend_operators.c Outdated
Comment thread Zend/zend_operators.c Outdated
@lazerg
lazerg requested a review from dstogov as a code owner August 7, 2026 14:21
@arnaud-lb
arnaud-lb merged commit 77170ee into php:PHP-8.4 Aug 10, 2026
18 checks passed
arnaud-lb added a commit that referenced this pull request Aug 10, 2026
* PHP-8.5:
  Add a stack limit check in zend_hash_compare() (#23090)
pull Bot pushed a commit to littlekign/php-src that referenced this pull request Aug 10, 2026
* PHP-8.4:
  Add a stack limit check in zend_hash_compare() (php#23090)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants